chore(maintenance): remove DOM lib, use undici-types for fetch types#5351
chore(maintenance): remove DOM lib, use undici-types for fetch types#5351svozza wants to merge 3 commits into
Conversation
…fetch types Closes #5350
powertools-for-aws-oss-automation
Bot
added
size/L
|
|
||
| app.post('/todos', async ({ req }) => { | ||
| const body = await req.json(); | ||
| const body = (await req.json()) as { title: string }; |
There was a problem hiding this comment.
I don't mind having these in the snippets, but I'm mildly concerned about the implications that these new types have on end customers.
Does this mean customers who use the library now have to do this cast every time they use this?
There was a problem hiding this comment.
Yeah, good point, there will be effects on customer. If you are doing stuff like req.json() in middleware or a handler like in this example, you would need to fix the types with a cast now that you are not getting Promise<any> back. I'm not sure that's a bad thing though, because it makes clear that you are handling untrusted data. Ultimately if you want real safety guarantees you should really be using a validation library like Zod and this problem goes away. That said, this does feel quite disruptive so I'm open to persuasion that we should leave things as they are.
There was a problem hiding this comment.
Let me take it for a spin and see how it works in practice before I insist further in either way.
powertools-for-aws-oss-automation
Bot
added
size/L
|
2 participants
Summary
Removes
DOMandDOM.Iterablefrom the roottsconfig.jsonlibso the codebase type-checks against Node.js types instead of browser types. Node'sfetch/Request/Response/Headersglobals now come from@types/node(backed byundici-types), whose stricterResponse.json(): Promise<unknown>surfaced ~50 previously unchecked response accesses in tests and snippets, which are now explicitly typed.Changes
tsconfig.jsonlibfrom["ES2024", "DOM", "DOM.Iterable", "ESNext.Disposable"]to["ES2024", "ESNext.Disposable"]and update the explanatory commentBodyInitas a type fromundici-typesinpackages/event-handler/src/http/converters.ts, since@types/nodedoes not expose it as a globalundici-typesas an explicit root devDependency (pinned to^7.24.6, matching the range@types/noderesolves) rather than relying on it being hoisted transitively via@types/node— placed at the root alongside@types/node, consistent with how the monorepo manages ambient Node typingsresponse.json()results in the event-handler e2e tests (httpRouter.test.ts) with the expected response shape of each routereq.json()toJSONValuein e2e/unit test route handlers so echoed request bodies satisfyHandlerResponsejson()results inexamples/snippets(advanced_fine_grained_responses.ts,customProviderVault.ts)document,window,localStorage, etc.) — none foundNote
On the
(await response.json()) as { ... }pattern: under undici-types,Response.json()returnsPromise<unknown>by design — bytes off the wire have no compile-time type, and DOM'sPromise<any>was silently disabling type checking at every one of these call sites. Some per-site type annotation is therefore unavoidable; the question is only where the trust lives. We considered afetchJson<T>helper (centralizes the cast),toMatchObjectassertions (no casts, but can't express numeric comparisons liketoBeGreaterThan), and zod parsing (real runtime validation, but adds a dependency and the most churn). We settled on the plain inline assertion as the standard, explicit idiom — each test documents the response shape it expects right where it's used.Issue number: closes #5350
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.
Disclaimer: We value your time and bandwidth. As such, any pull requests created on non-triaged issues might not be successful.